Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct and update comment on TTYPort #183

Merged
merged 3 commits into from
May 6, 2024

Conversation

Teufelchen1
Copy link
Contributor

Hey 🪼,

this aims to fix #180.
This change alters the comment around the TTYPort struct. It removes mentions of old functions/methods no longer in the code base. It adds a tiny example on how to open a tty port the correct way.

Additionally: Sneaks in a tiny fix for an spelling error further down the comment.

I am unsure how to test my change. I think it is important someone rebuilds the docs and checks that my code block is rendered as expected.

@EdJoPaTo
Copy link

EdJoPaTo commented May 6, 2024

I am unsure how to test my change. I think it is important someone rebuilds the docs and checks that my code block is rendered as expected.

cargo doc --no-deps --open

I would also expect no_run to be used here as the doc tests cargo test or cargo test --doc might be run on a machine without /dev/ttyS0?
Also see https://doc.rust-lang.org/rustdoc/write-documentation/documentation-tests.html.

This includes silencing a warning about port being unused.
Let's have more compact example and don't let an empty line slip in the
rendered example due to the empty lines before and after using 'master'
and 'slave' not being merged.
@sirhcel
Copy link
Contributor

sirhcel commented May 6, 2024

Thank you very much for your contribution @Teufelchen1 and the fruitful discussion @EdJoPaTo! Added no_run and cleaned up the example below.

Using

$ cargo doc --no-deps --open

generates the documentation. The tests for the documentation are run along with other tests. As of 777e15b:

$ export SERIALPORT_RS_TEST_PORT_1=DUMMY_1
$ export SERIALPORT_RS_TEST_PORT_2=DUMMY_2
$ cargo test --features ignore-hardware-tests
[...]
   Doc-tests serialport

running 4 tests
test src/lib.rs - new (line 804) - compile ... ok
test src/posix/tty.rs - posix::tty::TTYPort (line 40) - compile ... ok
test src/posix/tty.rs - posix::tty::TTYPort (line 50) ... ok
test src/posix/tty.rs - posix::tty::TTYPort::pair (line 252) ... ok

test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.63s

I'm currently working on changing the configuration for the tests so that when hardware tests are ignored no environment variables need to be set. Please be patient with this work-in-progress.

@sirhcel sirhcel merged commit 83fd7fb into serialport:main May 6, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Outdated documentation for TTYPort
3 participants